Hi @qpernil , thanks for your feedback on the code. At long last, I added EVP_DecryptFinal to the decryption. For AES-CCM it doesn't really have any benefit, as the OpenSSL example AES-CCM decryption code also mentions, finalization should not have any data for AES-CCM decryption.

I like the idea of an output formatted as a command-line input for yhwrap.

The only bottleneck is that I can't find any documentation where the delegated capabilities are stored in the object metadata. Without the delegated responsibilities, the command to reimport the key will not create an identical key to the one exported.

I'm open to advice: either we keep the current output and let the user convert how they see fit or we can create a partial input parameter list for yhwrap with the note that delegated capabilities have to be manually added. Or, if you can point me to something that documents how to get the delegated responsibilities, I'd be interested in implementing that too.

Hi @qpernil ,

I've run into another issue to fully recreate a yubihsm-wrap-compatible output. The yubihsm-wrap input is a PEM-encoded private key with some OID prefix, which is fine. But the yubihsm-unwrap output (the unwrapped key export) is the SHA512-hashed private key + public key. (64 bytes + 32 bytes)

I'm still looking at RFC8302 to see if I missed something. I'd be grateful for any advice on how to recreate the private key from the HSM export (unwrapped).

@greg-szabo if it's really the "expanded secret key" there's no way to recover the original seed. It would require a preimage attack on SHA-512.

Thanks, Tony, that's what I derived too. I'll have to find an Ed25519 library that can use this "expanded private key" as input.

@qpernil it seems that with the above knowledge, making yubihsm-unwrap create the input for yubihsm-wrap is not possible because of the difference of the PEM-encoded private key required for yubihsm-wrap and the SHA512-hashed private key returned by yubihsm-unwrap.

Unless there is a way to extract the private key (or seed as Golang defines it) from the HSM device, there's no way to recreate the exact PEM input for yubihsm-wrap.

With that said, I think that the exercise of yubihsm-unwrap is still valuable and unless you have other feedback on what to improve, it is ready for review and merging.

The YubiHSM currently only stores the hashed seed, so yubihsm-wrap would have to be modified to allow an alternative input format that contains the hashed seed instead, and then skip the SHA512 hashing it currently performs.

Hi @qpernil ,

I agree that extending yubihsm-wrap with an option to read hashed private keys for wrapping is a neat idea and it should be done to complement yubihsm-unwrap. Especially, because the current PEM-based import option in yubihsm-wrap is fairly cumbersome.

However, should we maybe do that in a separate PR? It seems to me that it's worth separating that work from this one which focuses on the possibilities with yubihsm-unwrap.

What do you think?

Yes that could certainly be done in a separate PR. Please also note #351

Yes, please, and thank you for #351 !! It took me a bunch of time to troubleshoot why some of my keys don't work and some do. The only hint came from RFC8032 but I didn't put it together for a long time.

The byte changes in #351 are non-reversible so at exporting we can't really do much about it and it's not a problem. If someone takes the key and mangles it again, it will result in the same key.

The other quirk that took me some time to realize is that the YubiHSM device must be using little-endian byte order and that's why the wrap command reverses the first 32 bytes (lines 150-154 in the same file).

Maybe during unwrap, we should reverse the reverse order of these bytes so it's easier for the user to use the output. (If I want to validate the key in a test where I have the seed key, I can use a simple sha512sum command to generate the hash but that's in the regular order on linux.) You have to have this inherent knowledge that the expanded secret key is little-endian ordered or we can simply turn it back.

Shall I add a byte reversal code to unwrap for this? I think this is the last minor point that's worth fixing for this specific PR.

The fact that it reverses the bytes is simply how the specification for ed25519 is written, the low 32 bytes of the SHA512 hash are to be interpreted as a little-endian integer (https://www.rfc-editor.org/rfc/rfc8032 section 5.1.5 for example), and the bit clearing & setting is also part of the specification. So the result is in fact a big-endian integer. I would suggest just repeating these steps on any conventional (i.e. non-hashed) key and then testing that the result is the same.

This took longer than expected, but I implemented byte reversal as the last step. With this, the output is as close to a SHA512 sum value as possible.

I have created a seed key and imported it into an HSM device, then exported the expanded secret key with yubihsm-unwrap. Both the seed key and the exported key provided the same signature using the Ed25519 encryption.

Is there anything else to add to this PR?

In general the format of keys depends on the type of key, for example you should only reverse the first 32 bytes if it is an ed25519 key. And even for those it really depends on what the purpose is of the unwrapping. As we have noted before it is not currently possible to get back to the seed for ed25519 keys. If you want to check that the unwrapped key matches a seed you should instead process the seed as yubihsm-wrap does, and then compare to the decrypted key. Likewise, for authentication keys that were derived from a password you can't get back the password.

Also, you seem to have some errors in the reversing code (array bounds), see the failed checks below.

Thanks for pointing out the errors. I think it's because I used array notation (brackets) on a pointer. I do a size check before the error message, so it should have worked but I rewrote it to pointer arithmetic so the warning doesn't trigger.

I was narrowly focusing on ed25119 because of my use case, but now that you mention it, yubihsm-wrap can work on other types than asymmetric key.

So, maybe, it's a better purpose for this code to showcase how to decrypt encrypted keys using the wrapkey but not make any assumptions on what key is decrypted. Do you think that makes more sense?

In that case, I'll have to remove the key reversal to fulfill the purpose.

After some contemplation, I admit you are right. This example application could decrypt any exported key, but only Ed25519 keys must be byte-reversed. So, I removed the byte reversal from the codebase.

With that said, the purpose of this application is to provide an example of how to decrypt exported keys using AES-CCM. I think having this in the library is useful, at least for completeness' sake.

Golang and Rust have no language-native AES-CCM decryption libraries. (In Golang they considered it, and they have other encryption schemes natively supported.) This C implementation with openssl is closer to a safe solution than the third-party libraries in other languages.

I merged main into this branch which may have broken CI. I'll check it tomorrow.

👋🏻 Howdy @greg-szabo! Thanks for implementing this, it saved me a ton of time and effort this weekend. I was successfully able to export and decrypt a secp256k1 key from the device!

I noticed a few rough edges as I played with this branch, and I've opened a PR to fix them here: greg-szabo#1. If we plan to merge this, I hope you'll consider including them!

I think this is a useful application, but it would perhaps be even more useful if the output was in some standard format other applications can use. yubihsm-wrap is an attempt to do something like that (but in the other direction), by taking PEM formatted keys such as those created by openssl. For your info there is an upcoming release of the YubiHSM that does store the ed25519 seed, and hence also returns it when exporting/wrapping keys. So you could basically mirror what yubihsm-wrap does but in the other direction. Just my .02 cents on what this application could be used for..